Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

[tls] write data by small (1500 bytes) chunks, see http://www.belshe.com/ #961

Closed
wants to merge 2 commits into from

Conversation

indutny
Copy link
Member

@indutny indutny commented Apr 21, 2011

This pull request was previously posted here, but this time it's low-level:

P.S.
See http://www.belshe.com/2010/12/17/performance-and-the-tls-record-size/

@ry
Copy link

ry commented Apr 21, 2011

benchmark?

@indutny
Copy link
Member Author

indutny commented Apr 21, 2011

Sorry, no benchmarks. That is optimization for server client's - read article carefully! (Last time I'd posted that pull request - I missed that fact)

@indutny
Copy link
Member Author

indutny commented Apr 21, 2011

Basically, if you're sending large chunks - browser can display parts of page before it stop receiving, b/c part of chunk can't be encrypted.

@indutny
Copy link
Member Author

indutny commented Apr 21, 2011

BTW, try this in Chrome:
https://donnerjack13589.dyndns.tv:8081/

It's nodejs SPDY server :) And it's sending data in 1500 bytes chunks

@pquerna
Copy link

pquerna commented Apr 29, 2011

I'm concerned about the effect this might have on throughput, and thats what it would be nice to have a benchmark of -- showing the effect is negligible.

1500 byte writes to SSL_write also would cause a bad behavior for many connections, because that would become 1500+ bytes with the TLS headers/protocol, and for most MTUs would end up being 2 packets instead of 1. (good thing to look at in wireshark over non-localhost)

If the benchmark shows this isn't a huge drop in throughput, I'd be interested in seeing what happens with using ~1300-1400 byte writes, which should drop you bellow the common MTUs.

@indutny
Copy link
Member Author

indutny commented Apr 29, 2011

Hm... Nice point, @pquerna.

Mike has told me to use 1400 bytes, but I'd seen 1500 in chromium's flip_server - so I decided to use last one...
Probably that was wrong step.

Can you please help and perform some tests?

@indutny
Copy link
Member Author

indutny commented Dec 7, 2011

I think we should come back to this issue, splitting can't be done in userland

@indutny
Copy link
Member Author

indutny commented Jul 13, 2012

Reconsider?

@Nodejs-Jenkins
Copy link

Can one of the admins verify this patch?

@indutny
Copy link
Member Author

indutny commented Apr 18, 2013

fuck it.

@indutny indutny closed this Apr 18, 2013
@isaacs
Copy link

isaacs commented Apr 18, 2013

fuck it.

@indutny you should be more friendly to pull request submitters ;P

@AlexeyKupershtokh
Copy link

Is it a special command for jenkins?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants